feat: implement server versioning approach with semantic versioning support #296

domdomegg · 2025-08-22T01:45:07Z

This implements the versioning approach agreed upon in issue #158:

Key Changes

Documentation

Added comprehensive versioning guide (docs/versioning.md)
Updated FAQ with versioning guidance and best practices
Enhanced schema description with SHOULD recommendations

Schema Updates

Added 255-character limit for version strings in schema.json
Enhanced version field description with semantic versioning guidance

Implementation

New isSemanticVersion() function for proper semver detection
Enhanced compareSemanticVersions() with prerelease support
Implemented compareVersions() strategy:
- Both semver: use semantic comparison
- Neither semver: use timestamp comparison
- Mixed: semver versions always take precedence
Updated publish logic to determine "latest" using new strategy
Added version length validation (255 char max)

Versioning Strategy

Version MUST be string up to 255 characters
SHOULD use semantic versioning for predictable ordering
SHOULD align with package versions to reduce confusion
MAY use prerelease labels for registry-specific versions
Registry attempts semver parsing, falls back to timestamp ordering
Clients SHOULD follow same comparison logic

Resolves #158

Generated with Claude Code

…upport This implements the versioning approach agreed upon in issue #158: ## Key Changes ### Documentation - Added comprehensive versioning guide (docs/versioning.md) - Updated FAQ with versioning guidance and best practices - Enhanced schema description with SHOULD recommendations ### Schema Updates - Added 255-character limit for version strings in schema.json - Enhanced version field description with semantic versioning guidance ### Implementation - New isSemanticVersion() function for proper semver detection - Enhanced compareSemanticVersions() with prerelease support - Implemented compareVersions() strategy: * Both semver: use semantic comparison * Neither semver: use timestamp comparison * Mixed: semver versions always take precedence - Updated publish logic to determine "latest" using new strategy - Added version length validation (255 char max) ### Versioning Strategy 1. Version MUST be string up to 255 characters 2. SHOULD use semantic versioning for predictable ordering 3. SHOULD align with package versions to reduce confusion 4. MAY use prerelease labels for registry-specific versions 5. Registry attempts semver parsing, falls back to timestamp ordering 6. Clients SHOULD follow same comparison logic Resolves #158 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: adam jones <[email protected]>

domdomegg · 2025-08-22T01:45:43Z

@claude I think the logic for ordering etc. shouldn't happen in database/memory.go - it should happen a layer higher at the service layer, not persistence layer.

claude · 2025-08-22T01:45:58Z

Claude finished @domdomegg's task —— View job

Successfully moved version ordering logic from database layer to service layer! ✅

Tasks Completed:

Analyze current versioning implementation in internal/database/memory.go
Examine service layer structure in internal/service/
Move version comparison logic to service layer
Update database layer to use service layer methods
Update any other references to the moved functions
Run linting and tests
Commit and push changes

Summary

Successfully refactored the version ordering logic as requested:

✅ Created internal/service/versioning.go with exported functions:

IsSemanticVersion() - checks if version follows semver format
CompareSemanticVersions() - compares two semver strings
CompareVersions() - implements the full versioning strategy

✅ Updated internal/service/registry_service.go:

Service layer now handles version comparison business logic in Publish() method
Added helper methods for getting existing versions and determining latest status

✅ Simplified internal/database/memory.go:

Removed version comparison functions (moved to service layer)
Simplified Publish() method to only handle persistence
Cleaned up unused imports

✅ All tests pass - No breaking changes to existing functionality

The architecture now properly separates concerns:

Service layer: Business logic (version comparison, determining latest)
Database layer: Pure persistence (storage, retrieval, constraints)

- Move isSemanticVersion, compareSemanticVersions, and compareVersions functions from internal/database/memory.go to new internal/service/versioning.go - Update service layer to handle version comparison business logic - Simplify database layer to only handle persistence - All tests pass, maintaining existing functionality Co-authored-by: adam jones <[email protected]>

domdomegg · 2025-08-22T02:54:20Z

docs/versioning.md

+
+### For Non-Semantic Versions
+If version parsing as semantic version fails:
+- The registry will always mark the version as latest (overriding any previous version)


This feels reasonable in isolation. but actually means that version's total ordering is inconsistent with is_latest, for packages with a mix of versions.

Not sure what is best to do here. Maybe this is fine. Maybe we should only update is_latest if it's now the latest in the total ordering spec below, idk.

I think this behavior is necessary because the publisher does not indicate whether they are actually using semantic versioning. For example, if we said "semantic versions always take precedence", and a publisher is using non-semantic version numbers that sometimes appear to be semantic, then they are going to have a bad time. 😆

Unfortunately, as you noted, this means there is no natural sort order. Instead, we have to maintain an insertion order based on this logic.

docs/versioning.md

internal/service/registry_service.go

internal/service/versioning.go

jonathanhefner · 2025-08-22T17:19:01Z

docs/versioning.md

+
+### For Non-Semantic Versions
+If version parsing as semantic version fails:
+- The registry will always mark the version as latest (overriding any previous version)


I think this behavior is necessary because the publisher does not indicate whether they are actually using semantic versioning. For example, if we said "semantic versions always take precedence", and a publisher is using non-semantic version numbers that sometimes appear to be semantic, then they are going to have a bad time. 😆

Unfortunately, as you noted, this means there is no natural sort order. Instead, we have to maintain an insertion order based on this logic.

docs/versioning.md

🏠 Remote-Dev: homespace

domdomegg · 2025-08-25T16:43:24Z

Requesting review from @rdimitrov :)

- Added versioning.go with semantic version comparison functions - Added versioning_test.go with comprehensive tests - Updated registry_service.go to validate version ordering on publish - Fixed nolint directives in publisher auth files :house: Remote-Dev: homespace

- Service layer determines if a version should be marked as latest - Database interface updated to accept isLatest parameter - UpdateLatestFlag method added for updating existing records - Memory and PostgreSQL implementations updated accordingly - Removed version comparison logic from database layer - All tests passing :house: Remote-Dev: homespace

- Database interface now accepts full RegistryMetadata object - More flexible for future extensions and metadata needs - Service layer creates and populates RegistryMetadata - Database implementations use provided metadata - Cleaner separation of concerns :house: Remote-Dev: homespace

- Service layer now generates the registry metadata ID - Database layer uses the provided ID instead of generating its own - Cleaner separation: service owns all business logic including ID generation - Database layer is purely for persistence :house: Remote-Dev: homespace

- Use registry metadata ID for both servers and server_extensions tables - Removes unnecessary serverID generation - Maintains 1:1 relationship between tables - Cleaner and more consistent with memory implementation :house: Remote-Dev: homespace

domdomegg · 2025-08-25T17:10:35Z

@claude can you review this?

claude · 2025-08-25T17:10:51Z

Claude finished @domdomegg's task —— View job

Code Review Summary

Comprehensive review of the versioning implementation with detailed feedback and recommendations.

🔍 Review Findings

✅ Strengths

Excellent Test Coverage - internal/service/versioning_test.go:153 provides comprehensive test cases covering edge cases, semantic versions, mixed scenarios, and error conditions
Clear Documentation - docs/versioning.md:135 provides thorough guidance with examples and best practices
Proper Architecture - Version comparison logic correctly moved to service layer as requested
Good Schema Validation - 255-character limit properly enforced in schema and database

⚠️ Critical Issues

1. Semantic Version Regex Too Permissive (`internal/service/versioning.go:10-45`)

The current implementation incorrectly identifies date formats like "2021.03.15" as valid semantic versions:

// Test shows: "2021.03.15" returns true, but this isn't valid semver
{"date format", "2021.03.15", true}, // Should be false

Issue: Date-based versions will be compared semantically instead of by timestamp, leading to incorrect ordering.

Fix Needed: Restrict semver validation to not allow leading zeros or extreme values that indicate date formats.

2. Inconsistent Version Ordering (Addresses review comment on `docs/versioning.md:71`)

The current behavior creates inconsistent is_latest flags:

Non-semantic version published after "1.0.0" → marked as latest
But "1.0.0" is still considered "higher" in semantic ordering
This creates confusion where is_latest=true doesn't align with logical version ordering

Impact: Client sorting will be inconsistent with registry's latest determination.

3. Prerelease Comparison Too Simplistic (`internal/service/versioning.go:85-92`)

Lexicographic comparison doesn't handle semantic version prerelease rules correctly:

// Current: "1.0.0-alpha.10" < "1.0.0-alpha.2" (incorrect)
// Should: "1.0.0-alpha.10" > "1.0.0-alpha.2"

🔧 Moderate Issues

4. Missing Version Length Validation (`internal/service/registry_service.go:71-146`)

Service layer doesn't validate 255-character limit before processing.

5. Inconsistent Error Handling

Service layer parsing errors in timestamp conversion on line 106 are silently ignored.

6. Fixed Limit in Service Layer (`internal/service/registry_service.go:92`)

Hardcoded 1000 limit noted in review - acceptable for now but could cause issues with servers having many versions.

📚 Documentation Issues

7. Example Conflicts (Addresses review comments on `docs/versioning.md`)

JavaScript code blocks need proper syntax highlighting
Example "1.2.3-1" conflicts with prerelease precedence explanation
Mixed version behavior could use clearer explanation of trade-offs

🚀 Recommendations

Immediate Fixes:

Fix semver validation to reject date formats and invalid patterns
Add version length validation in service layer
Improve prerelease comparison using proper semver precedence rules
Add error handling for timestamp parsing

Consider for Future:

Version ordering strategy: Consider alternatives to current mixed-mode approach to reduce confusion
Performance optimization: Cache version comparisons for repeated operations
Stricter validation: Consider rejecting clearly invalid patterns during publish

📝 Code Quality

Architecture: ✅ Properly layered, good separation of concerns
Testing: ✅ Excellent coverage, but needs tests for edge cases identified above
Error Handling: ⚠️ Some silent failures, needs improvement
Performance: ✅ Acceptable for current scale
Maintainability: ✅ Well-structured, readable code

The implementation shows solid engineering practices but needs fixes for the semantic version validation and version comparison logic to ensure consistent behavior across different version types.

Overall Assessment: Good foundation with some critical fixes needed for production readiness.

domdomegg · 2025-08-25T17:19:31Z

Response to Claude review:

is wrong, that is a semver
accepted for now
will fix!
Add formatting and validation for free-form fields #272
this is fine
accepted

…enhance tests 🏠 Remote-Dev: homespace

rdimitrov

Overall it looks good 👍 I added a few comments but my main one is around the custom semver implementation (less code == less problems 😃)

rdimitrov · 2025-08-26T07:26:59Z

internal/service/versioning.go

@@ -0,0 +1,180 @@
+package service


Shouldn't we consider using semver instead? It's from the official package - https://pkg.go.dev/golang.org/x/mod/semver and would save us from maintaining/testing that part

Yeah, that is probably better.

There are some Anthropic-internal reasons why I didn't do this initially, but realistically this is definitely the better approach.

rdimitrov · 2025-08-26T08:36:31Z

internal/service/registry_service.go

+			existingTime, _ := time.Parse(time.RFC3339, server.RegistryMetadata.ReleaseDate)
+
+			// Compare versions using the proper versioning strategy
+			comparison := CompareVersions(newVersion, existingVersion, currentTime, existingTime)


Should we exit early here in case the request is trying to publish a version that is already there (which I think we don't want to allow) or older?

Good point! I think publishing old versions is fine, but publishing duplicate version is definitely a no-no. Fixing...

rdimitrov

LGTM! 🚀

domdomegg commented Aug 22, 2025

View reviewed changes

docs/versioning.md Outdated Show resolved Hide resolved

domdomegg commented Aug 22, 2025

View reviewed changes

internal/service/registry_service.go Outdated Show resolved Hide resolved

domdomegg commented Aug 22, 2025

View reviewed changes

internal/service/versioning.go Show resolved Hide resolved

domdomegg marked this pull request as ready for review August 22, 2025 02:58

jonathanhefner reviewed Aug 22, 2025

View reviewed changes

domdomegg added 4 commits August 23, 2025 03:41

Address comments

4eb1635

🏠 Remote-Dev: homespace

fix: clarify version ordering rules for registry clients

15574c9

🏠 Remote-Dev: homespace

Merge main branch and migrate to PostgreSQL

96058de

🏠 Remote-Dev: homespace

Merge main branch and resolve conflicts

fb73fb6

🏠 Remote-Dev: homespace

domdomegg added 5 commits August 25, 2025 16:50

Implement prerelease version comparison according to semver spec and …

595edf0

…enhance tests 🏠 Remote-Dev: homespace

rdimitrov reviewed Aug 26, 2025

View reviewed changes

Address review comments

67e3c04

rdimitrov reviewed Aug 26, 2025

View reviewed changes

domdomegg merged commit 7b211a4 into main Aug 26, 2025
6 checks passed

domdomegg deleted the claude/issue-158-20250821-2216 branch August 26, 2025 13:55

feat: implement server versioning approach with semantic versioning support #296

feat: implement server versioning approach with semantic versioning support #296

Conversation

domdomegg commented Aug 22, 2025

Key Changes

Documentation

Schema Updates

Implementation

Versioning Strategy

Uh oh!

domdomegg commented Aug 22, 2025

Uh oh!

claude bot commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks Completed:

Summary

Uh oh!

domdomegg Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

jonathanhefner Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathanhefner Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domdomegg commented Aug 25, 2025

Uh oh!

domdomegg commented Aug 25, 2025

Uh oh!

claude bot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

✅ Strengths

⚠️ Critical Issues

1. Semantic Version Regex Too Permissive (internal/service/versioning.go:10-45)

2. Inconsistent Version Ordering (Addresses review comment on docs/versioning.md:71)

3. Prerelease Comparison Too Simplistic (internal/service/versioning.go:85-92)

🔧 Moderate Issues

4. Missing Version Length Validation (internal/service/registry_service.go:71-146)

5. Inconsistent Error Handling

6. Fixed Limit in Service Layer (internal/service/registry_service.go:92)

📚 Documentation Issues

7. Example Conflicts (Addresses review comments on docs/versioning.md)

🚀 Recommendations

Immediate Fixes:

Consider for Future:

📝 Code Quality

Uh oh!

domdomegg commented Aug 25, 2025

Uh oh!

rdimitrov left a comment

Choose a reason for hiding this comment

Uh oh!

rdimitrov Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

domdomegg Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

rdimitrov Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

domdomegg Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

rdimitrov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude bot commented Aug 22, 2025 •

edited

Loading

claude bot commented Aug 25, 2025 •

edited

Loading

1. Semantic Version Regex Too Permissive (`internal/service/versioning.go:10-45`)

2. Inconsistent Version Ordering (Addresses review comment on `docs/versioning.md:71`)

3. Prerelease Comparison Too Simplistic (`internal/service/versioning.go:85-92`)

4. Missing Version Length Validation (`internal/service/registry_service.go:71-146`)

6. Fixed Limit in Service Layer (`internal/service/registry_service.go:92`)

7. Example Conflicts (Addresses review comments on `docs/versioning.md`)